Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merge beta 21.3.0.2 into trunk #19742

Merged
merged 24 commits into from
Dec 6, 2022
Merged

Merge beta 21.3.0.2 into trunk #19742

merged 24 commits into from
Dec 6, 2022

Conversation

mokagio
Copy link
Contributor

@mokagio mokagio commented Dec 6, 2022 •
edited by twstokes
Loading

Includes:

@twstokes 🤔 I'm not sure at this point whether my amendment of the release notes should be considered valid, in light of the feature flag? Maybe better to remove it? Thanks!

I'm aware we might not ship 21.3, but just want to have the state in the codebase internally consistent. Also, if we revert my change before merging, we'll avoid confusion with the translators.

dvdchr and others added 21 commits December 5, 2022 18:24
@dvdchr
Do not present CMF when the feature flag is disabled
3049c72
@salimbraksa
Track Load WordPress screen events and content export events (#19719)
3b1cd08
@mokagio
Add note regarding #19714 to editorialized release notes
fcd38db
@mokagio
Update metadata strings
f6efa1d
@dvdchr
Merge pull request #19730 from wordpress-mobile/fix/show-login-when-c…
712cdf1 
…mf-flag-off
@twstokes
Revert "Enable content migration flow feature flags."
ae2fb32 
This reverts commit dd1ecb8.
@twstokes
Revert "Add release notes."
45e556b 
This reverts commit 440b8e8.
@staskus
Disable jetpackMigrationPreventDuplicateNotifications Feature Flag
e94bcee
@staskus
Revert changes in RELEASE-NOTES.txt
ac5fc43
@twstokes
Put the ContentMigrationCoordinator process behind the Content Migrat…
58f33a0 
…ion feature flag.
@twstokes
Revamp showUI logic and factor in content migration feature flag for …
493c621 
…Jetpack.
@twstokes
Merge pull request #19734 from wordpress-mobile/task/disable-cmf
e9a03e3 
[Jetpack Content Migration] Disable Content Migration Flow
@twstokes
Merge remote-tracking branch 'origin/release/21.3' into task/migratio…
d0dded9 
…n-coordinator-flag-check
@twstokes
Add immediate returns to improve readability.
85f4bfb
@twstokes
Merge pull request #19736 from wordpress-mobile/task/migration-coordi…
a4de7fe 
…nator-flag-check

[Jetpack Content Migration Flow] Put the ContentMigrationCoordinator behind feature flag
@twstokes
Merge remote-tracking branch 'origin/release/21.3' into task/disable-…
a325d3d 
…prevent-notifications
@mokagio
Update WordPress 21.3 release notes with Jetpack migration (#19732)
dfa3cda
@mokagio
Update app translations – Localizable.strings
60a1e32
@mokagio
Update WordPress metadata translations
0253634
@mokagio
Update Jetpack metadata translations
24fa043
@mokagio
Bump version number
55b975a
@mokagio mokagio requested review from twstokes and a team December 6, 2022 11:55
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Dec 6, 2022 •
edited
Loading

You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19742-fa14718 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Dec 6, 2022 •
edited
Loading

You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19742-fa14718 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

twstokes
twstokes reviewed Dec 6, 2022
View reviewed changes
Copy link
Contributor

@twstokes twstokes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but just want to have the state in the codebase internally consistent. Also, if we revert my change before merging, we'll avoid confusion with the translators.

Hey 👋 @mokagio - those sound like good reasons to me to revert the commit. Good catch. 👍

If I should open a PR to revert it just let me know. 🙇

staskus and others added 2 commits December 6, 2022 15:39
@staskus
Do not depend on remote feature flags
75c637f
@mokagio
Revert "Add note regarding #19714 to editorialized release notes"
27964b2 
This reverts commit fcd38db.

Since making that change to the release notes, the content migration
feature has been disabled (via feature toggle, see #19734, #19736) and
it would be misleading to advertise it in the release notes.

This also reverts f6efa1d.
@mokagio
Copy link
Contributor Author

mokagio commented Dec 6, 2022

If I should open a PR to revert it just let me know. 🙇

No need, done in 27964b2.

Sorry, I was only looking for your confirmation I didn't mean to suggest you'd have to make the change.

@mokagio mokagio marked this pull request as ready for review December 6, 2022 14:03
@mokagio mokagio enabled auto-merge December 6, 2022 14:04
twstokes
twstokes approved these changes Dec 6, 2022
View reviewed changes
Copy link
Contributor

@twstokes twstokes left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great @mokagio! 🙇

We're really close to merging #19735 (just waiting for all green tests). Would it be more efficient to include it on this merge, or is it better to wait?

Well, I may have answered my own question by merging the branch. 😬 Sorry if this complicated things. I've added #19735 to the description of this PR.

@twstokes
Merge pull request #19735 from wordpress-mobile/task/disable-prevent-…
fa14718 
…notifications

[Jetpack Focus] Revert Jetpack Notifications Disabling Feature Flag and Release Notes
@peril-wordpress-mobile
Copy link

Messages
📖 This PR has the 'Releases' label: some checks will be skipped.

Generated by 🚫 dangerJS

@mokagio
Copy link
Contributor Author

mokagio commented Dec 6, 2022 •
edited
Loading

@twstokes, no worries! 😄

Would it be more efficient to include it on this merge, or is it better to wait?

From the point of view of getting the changes into trunk, merging here is the most efficient thing to do because it'll mean we won't have to do another dedicated PR for it.

However, from the point of view of having a beta with those changes, merging the code will not trigger a new beta deployment. That is a process release managers (or anyone with the right tokens in their environment) trigger manually via bundle exec fastlane new_beta_release.

If you'd like a new beta with these changes, it'd be straightforward to make one but I'd suggest waiting until tomorrow (or ~12-24 hours) so that we can get more translations in.

@twstokes
Copy link
Contributor

twstokes commented Dec 6, 2022

@twstokes, no worries! 😄

Would it be more efficient to include it on this merge, or is it better to wait?

From the point of view of getting the changes into trunk, merging here is the most efficient thing to do because it'll mean we won't have to do another dedicated PR for it.

However, from the point of view of having a beta with those changes, merging the code will not trigger a new beta deployment. That is a process release managers (or anyone with the right tokens in their environment) trigger manually via bundle exec fastlane new_beta_release.

Whew, thanks a bunch @mokagio.

@mokagio mokagio merged commit eb7da67 into trunk Dec 6, 2022
@mokagio mokagio mentioned this pull request Dec 7, 2022
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@twstokes twstokes twstokes approved these changes

Assignees
No one assigned
Labels
Releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mokagio @wpmobilebot @twstokes @dvdchr @salimbraksa @staskus